Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add logging #1042

Merged
merged 15 commits into from
Oct 4, 2024
Merged

feat: Add logging #1042

merged 15 commits into from
Oct 4, 2024

Conversation

Vinzent03
Copy link
Collaborator

@Vinzent03 Vinzent03 commented Sep 24, 2024 •
edited
Loading

What kind of change does this PR introduce?

feature

What is the current behavior?

When debugging the auth broadcasting or the realtime/stream fixed I had to manually add many print statements, which was annoying. The js sdk already has some logging.

What is the new behavior?

Add logging via the logging package from dart.dev. This primarily only tracks the logs. When debug is true in the supabase_flutter Supabase instance, records with level INFO or higher are printed.
The logger instances are laid in a hierarchy separated by dots, which allows all supabase loggers to be united in a supabase logger instance.

@Vinzent03
Copy link
Collaborator Author

@dshukertjr For now, I've only added logging to the gotrue package. What do you think of this? If you consent, I would add this to the rest of the packages.

@dshukertjr
Copy link
Member

This is awesome! I love it so much!

@Vinzent03
Copy link
Collaborator Author

I've noticed that we should be careful with what data we log, especially sensitive data. So I've come up with the idea that records with level FINEST contain sensitive data like session data, or which postgrest urls are called. So that those records shouldn't be sent to sentry or similar without making such things clear in privacy notes. Let me know what you think of this approach and the documentation in the README of the supabase_flutter package, so I can add that section to the other packages as well.

@Vinzent03 Vinzent03 marked this pull request as ready for review October 4, 2024 12:00
dshukertjr
dshukertjr approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@dshukertjr dshukertjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

packages/gotrue/lib/src/gotrue_client.dart Show resolved Hide resolved
@Vinzent03
Copy link
Collaborator Author

I see now that there is no documentation in the README of the rest of the packages. So I would leave it for now only in the supabase_flutter package and you? or I? add it to the supabase website documentation.

@Vinzent03 Vinzent03 merged commit d1ecabd into main Oct 4, 2024
21 checks passed
@Vinzent03 Vinzent03 deleted the feat/logger branch October 4, 2024 22:55
@Vinzent03 Vinzent03 mentioned this pull request Oct 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dshukertjr dshukertjr dshukertjr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Vinzent03 @dshukertjr